[KERNEL] Add e2e tests for writing tables with collated columns #5357
Conversation
ilicmarkodb
force-pushed
the
add_e2e_tests_for_collation_write
branch
5 times, most recently
from
53c0334 to
9e24ec3
Compare
ilicmarkodb
force-pushed
the
add_e2e_tests_for_collation_write
branch
2 times, most recently
from
c05daf9 to
13e75c4
Compare
ilicmarkodb
force-pushed
the
add_e2e_tests_for_collation_write
branch
11 times, most recently
from
f8839d0 to
e95e836
Compare
ilicmarkodb
force-pushed
the
add_e2e_tests_for_collation_write
branch
from
e95e836 to
bc298a1
Compare
| return equals(dataType); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Is it worth having this + the other equivalent method or might they satisfy the same goal?
There was a problem hiding this comment.
I think equivalentIgnoreCollations is needed because of the cases like https://github.com/delta-io/delta/pull/5357/files#diff-c9fd0f3e881617ea0f2439a29c32a35e8c32fbcdda229105be76ece4001819acR200. Here we don't to ignore names and metadata.
|
|
||
| ColumnarBatch data = filteredBatch.getData(); | ||
| if (!data.getSchema().equals(tableSchema)) { | ||
| if (!data.getSchema().equivalentIgnoreCollations(tableSchema)) { |
There was a problem hiding this comment.
I left a similar comment on the other PR, but how do we know when/where we should use this instead? How can we be sure all the instances of comparison we do throughout the code-base won't now be an issue? concerned there might be somewhere that would fail only if a test had collations in the schema, which the majority of existing tests won't
It seems like we need to audit everywhere we might compare data types or schemas?
There was a problem hiding this comment.
There was a problem hiding this comment.
I'm worried we do this type of comparison all over..
ilicmarkodb
force-pushed
the
add_e2e_tests_for_collation_write
branch
from
c165c05 to
bb11dc1
Compare
ilicmarkodb
force-pushed
the
add_e2e_tests_for_collation_write
branch
from
bb11dc1 to
114903b
Compare
2 participants
Which Delta project/connector is this regarding?
Description
Added e2e tests for writing tables with collated columns. To enable writing collated data, the schema comparison had to be modified in some places.
How was this patch tested?
New tests.
Does this PR introduce any user-facing changes?
Yes, users can now create and write to Delta tables with collated types.